fix: rework nilsafe logic with subProperty ops #210
Conversation
| @@ -961,21 +962,19 @@ func TestExpr_nil_safe(t *testing.T) { | |||
|
|
|||
| func TestExpr_nil_safe_first_ident(t *testing.T) { | |||
There was a problem hiding this comment.
This is still a valid use case. If foo? is nil the entire chain should be considered safe
There was a problem hiding this comment.
not really, just type foo and you will get an error unknown name foo, to avoid this error you can use AllowUndefinedVariables option. (or we can support something like ?.foo)
foo?.bar - here operator ?. should be affect bar property only
foo.bar?.baz - and here only on baz, bar (like foo above) isn't nilsafe
There was a problem hiding this comment.
It's not about foo being the first variable, it's about the rest of the chain being safe after the nil operator.
If you have foo.missing?.test.err having missing be nil should keep the rest of the chain safe, a nil test shouldn't cause the compiler to error on err unless missing is not nil
There was a problem hiding this comment.
IMO not supporting the nil operator on the first identifier would be a weird inconsistency but at the very least you need to support this use case: https://github.com/antonmedv/expr/pull/211/files#diff-c7a89724039da4beba41207c0d28a3d27c3474ceee39acedf5ff5eb3207691e4R997
There was a problem hiding this comment.
hm, maybe we need to add option to js-like fetch from objects?
in go getting an unknown property is forbidden and returned error
in js getting an unknown property returned undefined
#173 (comment)
fix for that just change scope for nilsafe variable :)
also seems IdentifierNode shouldn't have NilSafe option